-
Notifications
You must be signed in to change notification settings - Fork 120
chrdev: Instead of passing a Range of minors, default to starting at 0 #108
base: master
Are you sure you want to change the base?
Conversation
Since we're using a builder pattern we can actually implement defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be cool to have a test for this, probably in chrdev-region-allocation
if !name.ends_with('\x00') { | ||
return Err(Error::EINVAL); | ||
} | ||
|
||
return Ok(Builder { | ||
name, | ||
minors, | ||
0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0, | |
minor_start: 0, |
file_ops: Vec<&'static FileOperationsVtable>, | ||
} | ||
|
||
impl Builder { | ||
pub fn start_minor_at(&mut self, minor_start: u16) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should take self
and return Buider
, like register_device
does.
Based on #110, I think we do need to support allocating N minors without actually registering devices on them. I think the best way to do that is a second builder method |
I don't totally understand what binderfs is doing with chrdevs. It looks it's calling I don't think that "allocate N chrdevs with operations, and M more without" is quite right, because
Two possible ideas for what to do with the API:
But I'm not totally sure I understand what a good API looks like, here. |
Since we're using a builder pattern we can actually implement defaults.